CMake and testability improvements #308
Conversation
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
Signed-off-by: Vladislav Shchapov <[email protected]>
|
@phprus Maybe it will be good to add and install CMake target file |
| # Force to always compile with W4 | ||
| if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") | ||
| string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") | ||
| else() | ||
| set(ARGPARSE_PEDANTIC_COMPILE_FLAGS "/W4") | ||
| endif() |
There was a problem hiding this comment.
Isn't just putting /W4 will guarantee it will always replace other lower level warning level?
| elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU") | ||
| set(ARGPARSE_PEDANTIC_COMPILE_FLAGS -Wall -Wno-long-long -pedantic -Wsign-conversion -Wshadow -Wconversion -Werror -Wextra) | ||
| elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") | ||
| set(ARGPARSE_PEDANTIC_COMPILE_FLAGS -Wall -Wno-long-long -pedantic -Wsign-conversion -Wshadow -Wconversion -Werror -Wextra) |
There was a problem hiding this comment.
I don't see any differences in here, why don't we use this instead:
| elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU") | |
| set(ARGPARSE_PEDANTIC_COMPILE_FLAGS -Wall -Wno-long-long -pedantic -Wsign-conversion -Wshadow -Wconversion -Werror -Wextra) | |
| elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") | |
| set(ARGPARSE_PEDANTIC_COMPILE_FLAGS -Wall -Wno-long-long -pedantic -Wsign-conversion -Wshadow -Wconversion -Werror -Wextra) | |
| elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") | |
| set(ARGPARSE_PEDANTIC_COMPILE_FLAGS -Wall -Wno-long-long -pedantic -Wsign-conversion -Wshadow -Wconversion -Werror -Wextra) |
| working-directory: ${{runner.workspace}}/build | ||
| shell: bash | ||
| run: cmake -DARGPARSE_BUILD_SAMPLES=ON -DCMAKE_BUILD_TYPE=Debug ${{ matrix.cmake_opts }} $GITHUB_WORKSPACE |
There was a problem hiding this comment.
I think it's better to leave it like the previous implementation, no need to add another step for creating a directory:
| working-directory: ${{runner.workspace}}/build | |
| shell: bash | |
| run: cmake -DARGPARSE_BUILD_SAMPLES=ON -DCMAKE_BUILD_TYPE=Debug ${{ matrix.cmake_opts }} $GITHUB_WORKSPACE | |
| run: cmake . -B build -DARGPARSE_BUILD_SAMPLES=ON -DCMAKE_BUILD_TYPE=Debug ${{ matrix.cmake_opts }} |
Also setting shell: bash is not needed here.
| working-directory: ${{runner.workspace}}/build | ||
| run: cmake --build . --config Debug --verbose |
There was a problem hiding this comment.
No need to set the working-directory, we can just use this:
| working-directory: ${{runner.workspace}}/build | |
| run: cmake --build . --config Debug --verbose | |
| run: cmake --build build --config Debug --verbose |
| working-directory: ${{runner.workspace}}/build | ||
| run: ctest -C Debug -V | ||
| env: | ||
| CTEST_OUTPUT_ON_FAILURE: True |
There was a problem hiding this comment.
The same here, we can also use --test-dir instead of setting the working-directory and use --output-on-failure instead of CTEST_OUTPUT_ON_FAILURE:
| working-directory: ${{runner.workspace}}/build | |
| run: ctest -C Debug -V | |
| env: | |
| CTEST_OUTPUT_ON_FAILURE: True | |
| run: ctest --test-dir build -C Debug -V --output-on-failure |
CMake and testability improvements.
I apologize for the large number of changes in single PR.
Purpose of changes:
argparseis included viaadd_subdirectory.CMakeLists.txtproject files.@p-ranav, Please review my PR.